Skip to content

Conversation

@joshheald
Copy link
Contributor

@joshheald joshheald commented Oct 28, 2022

Part of: #7855

Not addressed: UTM parameters, analytic events

Description

JITMs contain a link for the CTA button, which we should open in a webview to allow the merchant to take action on a given JITM.

This PR extracts the existing logic for opening a webview from the upsell card readers campaign, and generalises it into a WooNavigationSheet, used by WebViewSheet, which will contain either an AuthenticatedWebView or a WebView.

When the user taps the CTA for a message, the WebView is presented in a sheet, which can be dismissed with a swipe or by tapping Done. The title of the message is used for the WebViewSheet title to provide context.

woocommerce.com and wordpress.com links will be opened in an AuthenticatedWebView, which sets the appropriate authentication so that log in to these sites is more convenient. For users with 2FA, entering the code happens at the start of the flow, which isn't ideal but is a web behaviour, that the apps can't change (other than by using a non-authenticated webview.)

Testing instructions

JITM on my store

N.B. see pdfdoF-1uc-p2#comment-2581 for details of setting up your store to be eligible for the test JITM

With a US store that is eligible for the test JITM, on a debug/alpha build of the app

  1. Open the app
  2. Observe that the message is shown
  3. Tap the CTA
  4. Observe that the webview is opened and takes you to the M2 reader page, and that when you log in you only need to authorise your wpcom account, not enter the credentials.

Upsell Card Readers cards

The Upsell Card Reader campaigns have all been updated to use the extracted web views, so could do with being tested again.

In a US or CA WCPay store

  1. Open the app and go to the Upsell Card Reader card
  2. Tap the Purchase card reader button
  3. Observe that the webview is opened and takes you to the M2/WisePad 3 reader page, and that when you log in you only need to authorise your wpcom account, not enter the credentials.

Locations of the card: top of Order List, and the Payment Method screen (after tapping Take Payment)

See #7862 for more detailed testing instructions.

Screenshots

cta-on-jitm.mp4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@joshheald joshheald added type: enhancement A request for an enhancement. feature: jitm Related to Just In Time Messages labels Oct 28, 2022
@joshheald joshheald added this to the 11.1 milestone Oct 28, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 28, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr7973-89d238e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@joshheald joshheald marked this pull request as ready for review October 28, 2022 16:06
@joshheald joshheald requested review from pachlava and toupper October 28, 2022 16:07
@pachlava
Copy link
Contributor

@joshheald 👋

I can see that this is taking place on your video recording as well, so it's probably expected. I'm just wondering if authorizing the WPCOM account on every opening of a webview has technical reasons which are hard to overcome? (if opening a webview is similar to starting a browser session from scratch every time, then it's understood)

@pachlava
Copy link
Contributor

I noticed no issues on iPhone, but for iPad there's one. The webview, when initiated from Stats (JITM) or Orders List (Upsell CR), has a split view:

Screenshot 2022-10-31 at 10 39 52

Screenshot 2022-10-31 at 10 40 04

The viewbview looks perfect, though, when it's shown via collect payment flow (IPP / Order Detail):

Screenshot 2022-10-31 at 10 43 03

@joshheald
Copy link
Contributor Author

I noticed no issues on iPhone, but for iPad there's one. The webview, when initiated from Stats (JITM) or Orders List (Upsell CR), has a split view:

@pachlava Thanks, good catch, and that's a strange one! I suspect it's because it's being presented from a Split View in the Orders and Dashboard cases, but on payment methods it's presented from another modal. I'll dig in to it.

I'm just wondering if authorizing the WPCOM account on every opening of a webview has technical reasons which are hard to overcome? (if opening a webview is similar to starting a browser session from scratch every time, then it's understood)

I've not really tried to overcome the issues here, but my memory and understanding of web views is that we can't rely on keeping the session around between launches. It's potentially worth another project to dig in to the detail here and see whether we can avoid doing it every time, but I think that's outside the scope of this work.

Previously when the sheet was created on iPad, the content would be rendered as the master view of a split view, with no detail view. Often this would lead to no content being displayed, and a confusing back button added.

StackNavigationViewStyle makes no change to the iPhone design but makes this sheet suitable for form/page sheet presentation on iPad, showing the content immediately.
@joshheald
Copy link
Contributor Author

@pachlava iPad display issue fixed in c319e23.

Thanks for finding this – it is a pre-existing issue in the Order List.

ipad-no-split.mp4

@pachlava
Copy link
Contributor

@pachlava iPad display issue fixed in c319e23.

Thanks, @joshheald - I'm taking a look

@toupper toupper self-assigned this Oct 31, 2022
Copy link
Contributor

@pachlava pachlava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No complaints from my side, @joshheald: the fix for webview layout on iPda looks good to me 👍 🙂 Thanks!

One question: is it by design that webview title initiated from JITM is different from the one initiated from Upsell CR?

JITM:
Screenshot 2022-10-31 at 13 32 26

UCR:
Screenshot 2022-10-31 at 13 37 40

@joshheald
Copy link
Contributor Author

One question: is it by design that webview title initiated from JITM is different from the one initiated from Upsell CR?

@pachlava thanks for checking again so quickly.

hat is using the title of the JITM (i.e. the bold text displayed at the top of the JITM card) for the title of the webview too. Since the message could be anything, and we only have a limited number of fields to use, when I wrote it that seemed like the best one. The UCR campaign doesn't have this limitation....

However, I could use the button title instead, which actually would match the UCR implementation. I'll check what Anirudh did on Android, if anything, and mirror it in a future PR

@pachlava
Copy link
Contributor

Both UCR and JITM webivews on Android use Order Card Reader for title. I think it would be nice to have it aligned on iOS, because both are eventually about the same thing.

import class Networking.UserAgent

/// Mirror of AuthenticatedWebView, for equivalent display of URLs in `WKWebView` that do not need authentication on WPCom.
struct WebView: UIViewRepresentable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering, since most of this code is duplicated with AuthenticatedWebView, is there a way to collapse both structs into one with some sort of configuration to authenticate when necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toupper Probably... I'll take a look towards the end of the week in another PR, as I'm trying to de-risk delivery in 11.1 as much as possible. Other than that, would you be happy to approve?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great! sure thing 👍

# Conflicts:
#	WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewController.swift
#	WooCommerce/Classes/ViewRelated/Dashboard/DashboardViewModel.swift
@joshheald joshheald enabled auto-merge October 31, 2022 17:20
@joshheald joshheald merged commit 1fb1a1e into trunk Oct 31, 2022
@joshheald joshheald deleted the issue/7855-open-webview-for-jitm-cta branch October 31, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: jitm Related to Just In Time Messages type: enhancement A request for an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants